Add OAuth2 support in spring-config-client#3129
Conversation
2180ae6 to
b947caa
Compare
|
|
||
| private ClientRegistrationRepository clientRegistrationRepository( | ||
| ConfigClientProperties.OAuth2Properties properties) { | ||
| OAuth2ClientProperties oauth2ClientProperties = new OAuth2ClientProperties(); |
There was a problem hiding this comment.
I would think we would just use the existing properties
https://docs.spring.io/spring-security/reference/servlet/oauth2/login/core.html
There was a problem hiding this comment.
The reason I took this approach for a few reasons.
- First, to keep things isolated and not depend on Spring Security autoconfiguration, reuse the classes provided and imitate the same here.
- Additionally, I am not sure about how it would work, as this initialization happens so early in the app lifecycle.
- Also, there are scenarios where configserver would actually serve the
spring.security.oauth2.client.*
|
|
||
| List<ClientRegistration> registrations = new ArrayList<>( | ||
| new OAuth2ClientPropertiesMapper(oauth2ClientProperties).asClientRegistrations().values()); | ||
| return new InMemoryClientRegistrationRepository(registrations); |
There was a problem hiding this comment.
This would need to be configurable. Maybe by default we use InMemoryClientRegistrationRepository but this should be a bean that can be supplied
There was a problem hiding this comment.
Sure, if there is a usecase for this, as things are a bit tricky to wire things in with all BootstrapRegistryInitializer. If I could get a little more context on what is required I could try to address this
|
Thanks. I will need to set aside some time to deep dive on this after our GA release. Also would love @jgrandja to take a look as well. |
@ryanjbaxter Thanks for the review. Since the changes are not disruptive. So, I was hoping this would make it to 5.0.0. I do understand this is a bit of an ask given that GA is so close. If there is something I could do to make it a possibility, please do let me know. |
The RC1 release is already in progress and everyone is focused on that and then the GA after. It will have to wait, I'm afraid. |
|
If I have some time I will take a look at it but as Spencer said we have a lot on our plate between now and GA and we don't want to rush this, we want to make sure we get it right. Unfortunately this major has been a challenge for the Spring Cloud team as we try to keep up with the incoming changes from Spring Boot and Spring Framework. We did not have as much time as we would have liked to put in new features into Spring Cloud. |
|
@prafsoni I took a look at the PR and it introduces the |
@jgrandja Thanks for the review, Since, I didnot add anything that is not in spring-security I would expect it to be automatically addressed with the dependency upgrade. Please, do let me know if I need to take any additional steps. |
f871b37 to
b669ef0
Compare
b669ef0 to
f449b48
Compare
…ud config client Signed-off-by: prafsoni <prafsoni@gmail.com>
Signed-off-by: prafsoni <prafsoni@gmail.com>
…suer uri Signed-off-by: prafsoni <prafsoni@gmail.com>
Signed-off-by: prafsoni <prafsoni@gmail.com>
Signed-off-by: prafsoni <prafsoni@gmail.com>
Signed-off-by: prafsoni <prafsoni@gmail.com>
f449b48 to
244c06a
Compare
Move OAuth2 wiring into a new oauth2 sub-package gated by ClassUtils.isPresent so spring-boot-starter-security-oauth2-client is genuinely optional. Replace the duplicate spring.cloud.config.oauth2 registration/provider schema with a client-registration-id that points at a standard Spring Security registration under spring.security.oauth2.client.*. ConfigClientRequestTemplateFactory no longer references Spring Security; it just accepts additional interceptors. The interceptor is wired for both the Config Data flow (ConfigClientOAuth2Support.registerInterceptor via the bootstrap context, with the bind handler threaded so encrypted/placeholder secrets resolve) and the legacy bootstrap flow (ConfigClientOAuth2BootstrapConfiguration beans). Every collaborator (ClientRegistrationRepository, OAuth2AuthorizedClientManager, ClientRegistrationIdResolver, and the interceptor itself) is overridable, and the default repository is built lazily so a user-supplied manager makes it unnecessary. Signed-off-by: prafsoni <prafsoni@gmail.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@ryanjbaxter @spencergibb @jgrandja Have updated the implementation please have another look at the PR. Would be great if we could get this in along side boot 4.1.x release. |
|
Sorry, the earliest this would be released ga is November. What we were releasing next week is a patch release, not a feature release |
@spencergibb Would we be able to get this merged or will need to wait till Nov ? |
|
I can't really say when it would be merged |
|
Alright, please review and leave feedback when possible. Thanks |
Resolves #2348
spring-cloud-clientusingspring-security-oauth2-client.spring.cloud.config.oauth2.*.spring.cloud.config.oauth2.enabled: false) by default to maintain existing behavior.spring-cloud-config-client-oauth2-testsmodule for OAuth2 support integration tests